batman-adv: Merge bugfixes from 2024.4 1093/head
authorSven Eckelmann <[email protected]>
Tue, 10 Dec 2024 21:45:54 +0000 (22:45 +0100)
committerSven Eckelmann <[email protected]>
Tue, 10 Dec 2024 21:52:07 +0000 (22:52 +0100)
* Do not send uninitialized TT changes
* Remove uninitialized data in full table TT response
* Do not let TT changes list grows indefinitely

Signed-off-by: Sven Eckelmann <[email protected]>
batman-adv/Makefile
batman-adv/patches/0003-batman-adv-Do-not-send-uninitialized-TT-changes.patch [new file with mode: 0644]
batman-adv/patches/0004-batman-adv-Remove-uninitialized-data-in-full-table-T.patch [new file with mode: 0644]
batman-adv/patches/0005-batman-adv-Do-not-let-TT-changes-list-grows-indefini.patch [new file with mode: 0644]

index d38ec6d4c426ebaf004833f6d4ffd09e417fe834..219ca9f3a31269de202c119ea12013e4c1220778 100644 (file)
@@ -4,7 +4,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=batman-adv
 PKG_VERSION:=2024.3
-PKG_RELEASE:=2
+PKG_RELEASE:=3
 
 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
 PKG_SOURCE_URL:=https://downloads.open-mesh.org/batman/releases/batman-adv-$(PKG_VERSION)
diff --git a/batman-adv/patches/0003-batman-adv-Do-not-send-uninitialized-TT-changes.patch b/batman-adv/patches/0003-batman-adv-Do-not-send-uninitialized-TT-changes.patch
new file mode 100644 (file)
index 0000000..e2276c8
--- /dev/null
@@ -0,0 +1,64 @@
+From: Remi Pommarel <[email protected]>
+Date: Fri, 22 Nov 2024 16:52:48 +0100
+Subject: batman-adv: Do not send uninitialized TT changes
+
+The number of TT changes can be less than initially expected in
+batadv_tt_tvlv_container_update() (changes can be removed by
+batadv_tt_local_event() in ADD+DEL sequence between reading
+tt_diff_entries_num and actually iterating the change list under lock).
+
+Thus tt_diff_len could be bigger than the actual changes size that need
+to be sent. Because batadv_send_my_tt_response sends the whole
+packet, uninitialized data can be interpreted as TT changes on other
+nodes leading to weird TT global entries on those nodes such as:
+
+ * 00:00:00:00:00:00   -1 [....] (  0) 88:12:4e:ad:7e:ba (179) (0x45845380)
+ * 00:00:00:00:78:79 4092 [.W..] (  0) 88:12:4e:ad:7e:3c (145) (0x8ebadb8b)
+
+All of the above also applies to OGM tvlv container buffer's tvlv_len.
+
+Remove the extra allocated space to avoid sending uninitialized TT
+changes in batadv_send_my_tt_response() and batadv_v_ogm_send_softif().
+
+Fixes: 8405301b9794 ("batman-adv: tvlv - convert tt data sent within OGMs")
+Signed-off-by: Remi Pommarel <[email protected]>
+Signed-off-by: Sven Eckelmann <[email protected]>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/5fe3a7a48ea6374280dc855db7b802d70b1870c6
+
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -990,6 +990,7 @@ static void batadv_tt_tvlv_container_upd
+       int tt_diff_len, tt_change_len = 0;
+       int tt_diff_entries_num = 0;
+       int tt_diff_entries_count = 0;
++      size_t tt_extra_len = 0;
+       u16 tvlv_len;
+       tt_diff_entries_num = atomic_read(&bat_priv->tt.local_changes);
+@@ -1027,6 +1028,9 @@ static void batadv_tt_tvlv_container_upd
+       }
+       spin_unlock_bh(&bat_priv->tt.changes_list_lock);
++      tt_extra_len = batadv_tt_len(tt_diff_entries_num -
++                                   tt_diff_entries_count);
++
+       /* Keep the buffer for possible tt_request */
+       spin_lock_bh(&bat_priv->tt.last_changeset_lock);
+       kfree(bat_priv->tt.last_changeset);
+@@ -1035,6 +1039,7 @@ static void batadv_tt_tvlv_container_upd
+       tt_change_len = batadv_tt_len(tt_diff_entries_count);
+       /* check whether this new OGM has no changes due to size problems */
+       if (tt_diff_entries_count > 0) {
++              tt_diff_len -= tt_extra_len;
+               /* if kmalloc() fails we will reply with the full table
+                * instead of providing the diff
+                */
+@@ -1047,6 +1052,8 @@ static void batadv_tt_tvlv_container_upd
+       }
+       spin_unlock_bh(&bat_priv->tt.last_changeset_lock);
++      /* Remove extra packet space for OGM */
++      tvlv_len -= tt_extra_len;
+ container_register:
+       batadv_tvlv_container_register(bat_priv, BATADV_TVLV_TT, 1, tt_data,
+                                      tvlv_len);
diff --git a/batman-adv/patches/0004-batman-adv-Remove-uninitialized-data-in-full-table-T.patch b/batman-adv/patches/0004-batman-adv-Remove-uninitialized-data-in-full-table-T.patch
new file mode 100644 (file)
index 0000000..b5df5c1
--- /dev/null
@@ -0,0 +1,101 @@
+From: Remi Pommarel <[email protected]>
+Date: Fri, 22 Nov 2024 16:52:49 +0100
+Subject: batman-adv: Remove uninitialized data in full table TT response
+
+The number of entries filled by batadv_tt_tvlv_generate() can be less
+than initially expected in batadv_tt_prepare_tvlv_{global,local}_data()
+(changes can be removed by batadv_tt_local_event() in ADD+DEL sequence
+in the meantime as the lock held during the whole tvlv global/local data
+generation).
+
+Thus tvlv_len could be bigger than the actual TT entry size that need
+to be sent so full table TT_RESPONSE could hold invalid TT entries such
+as below.
+
+ * 00:00:00:00:00:00   -1 [....] (  0) 88:12:4e:ad:7e:ba (179) (0x45845380)
+ * 00:00:00:00:78:79 4092 [.W..] (  0) 88:12:4e:ad:7e:3c (145) (0x8ebadb8b)
+
+Remove the extra allocated space to avoid sending uninitialized entries
+for full table TT_RESPONSE in both batadv_send_other_tt_response() and
+batadv_send_my_tt_response().
+
+Fixes: 21a57f6e7a3b ("batman-adv: make the TT CRC logic VLAN specific")
+Signed-off-by: Remi Pommarel <[email protected]>
+Signed-off-by: Sven Eckelmann <[email protected]>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/095c3965bdc29e43546a9cdd21f179f952e01f48
+
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -2754,14 +2754,16 @@ static bool batadv_tt_global_valid(const
+  *
+  * Fills the tvlv buff with the tt entries from the specified hash. If valid_cb
+  * is not provided then this becomes a no-op.
++ *
++ * Return: Remaining unused length in tvlv_buff.
+  */
+-static void batadv_tt_tvlv_generate(struct batadv_priv *bat_priv,
+-                                  struct batadv_hashtable *hash,
+-                                  void *tvlv_buff, u16 tt_len,
+-                                  bool (*valid_cb)(const void *,
+-                                                   const void *,
+-                                                   u8 *flags),
+-                                  void *cb_data)
++static u16 batadv_tt_tvlv_generate(struct batadv_priv *bat_priv,
++                                 struct batadv_hashtable *hash,
++                                 void *tvlv_buff, u16 tt_len,
++                                 bool (*valid_cb)(const void *,
++                                                  const void *,
++                                                  u8 *flags),
++                                 void *cb_data)
+ {
+       struct batadv_tt_common_entry *tt_common_entry;
+       struct batadv_tvlv_tt_change *tt_change;
+@@ -2775,7 +2777,7 @@ static void batadv_tt_tvlv_generate(stru
+       tt_change = tvlv_buff;
+       if (!valid_cb)
+-              return;
++              return tt_len;
+       rcu_read_lock();
+       for (i = 0; i < hash->size; i++) {
+@@ -2801,6 +2803,8 @@ static void batadv_tt_tvlv_generate(stru
+               }
+       }
+       rcu_read_unlock();
++
++      return batadv_tt_len(tt_tot - tt_num_entries);
+ }
+ /**
+@@ -3076,10 +3080,11 @@ static bool batadv_send_other_tt_respons
+                       goto out;
+               /* fill the rest of the tvlv with the real TT entries */
+-              batadv_tt_tvlv_generate(bat_priv, bat_priv->tt.global_hash,
+-                                      tt_change, tt_len,
+-                                      batadv_tt_global_valid,
+-                                      req_dst_orig_node);
++              tvlv_len -= batadv_tt_tvlv_generate(bat_priv,
++                                                  bat_priv->tt.global_hash,
++                                                  tt_change, tt_len,
++                                                  batadv_tt_global_valid,
++                                                  req_dst_orig_node);
+       }
+       /* Don't send the response, if larger than fragmented packet. */
+@@ -3203,9 +3208,11 @@ static bool batadv_send_my_tt_response(s
+                       goto out;
+               /* fill the rest of the tvlv with the real TT entries */
+-              batadv_tt_tvlv_generate(bat_priv, bat_priv->tt.local_hash,
+-                                      tt_change, tt_len,
+-                                      batadv_tt_local_valid, NULL);
++              tvlv_len -= batadv_tt_tvlv_generate(bat_priv,
++                                                  bat_priv->tt.local_hash,
++                                                  tt_change, tt_len,
++                                                  batadv_tt_local_valid,
++                                                  NULL);
+       }
+       tvlv_tt_data->flags = BATADV_TT_RESPONSE;
diff --git a/batman-adv/patches/0005-batman-adv-Do-not-let-TT-changes-list-grows-indefini.patch b/batman-adv/patches/0005-batman-adv-Do-not-let-TT-changes-list-grows-indefini.patch
new file mode 100644 (file)
index 0000000..f8d8fe0
--- /dev/null
@@ -0,0 +1,63 @@
+From: Remi Pommarel <[email protected]>
+Date: Fri, 22 Nov 2024 16:52:50 +0100
+Subject: batman-adv: Do not let TT changes list grows indefinitely
+
+When TT changes list is too big to fit in packet due to MTU size, an
+empty OGM is sent expected other node to send TT request to get the
+changes. The issue is that tt.last_changeset was not built thus the
+originator was responding with previous changes to those TT requests
+(see batadv_send_my_tt_response). Also the changes list was never
+cleaned up effectively never ending growing from this point onwards,
+repeatedly sending the same TT response changes over and over, and
+creating a new empty OGM every OGM interval expecting for the local
+changes to be purged.
+
+When there is more TT changes that can fit in packet, drop all changes,
+send empty OGM and wait for TT request so we can respond with a full
+table instead.
+
+Fixes: 8405301b9794 ("batman-adv: tvlv - convert tt data sent within OGMs")
+Signed-off-by: Remi Pommarel <[email protected]>
+Acked-by: Antonio Quartulli <[email protected]>
+Signed-off-by: Sven Eckelmann <[email protected]>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/4d49d6e9d60c41a6da727108518cb8fb33295537
+
+--- a/net/batman-adv/translation-table.c
++++ b/net/batman-adv/translation-table.c
+@@ -990,6 +990,7 @@ static void batadv_tt_tvlv_container_upd
+       int tt_diff_len, tt_change_len = 0;
+       int tt_diff_entries_num = 0;
+       int tt_diff_entries_count = 0;
++      bool drop_changes = false;
+       size_t tt_extra_len = 0;
+       u16 tvlv_len;
+@@ -997,10 +998,17 @@ static void batadv_tt_tvlv_container_upd
+       tt_diff_len = batadv_tt_len(tt_diff_entries_num);
+       /* if we have too many changes for one packet don't send any
+-       * and wait for the tt table request which will be fragmented
++       * and wait for the tt table request so we can reply with the full
++       * (fragmented) table.
++       *
++       * The local change history should still be cleaned up so the next
++       * TT round can start again with a clean state.
+        */
+-      if (tt_diff_len > bat_priv->soft_iface->mtu)
++      if (tt_diff_len > bat_priv->soft_iface->mtu) {
+               tt_diff_len = 0;
++              tt_diff_entries_num = 0;
++              drop_changes = true;
++      }
+       tvlv_len = batadv_tt_prepare_tvlv_local_data(bat_priv, &tt_data,
+                                                    &tt_change, &tt_diff_len);
+@@ -1009,7 +1017,7 @@ static void batadv_tt_tvlv_container_upd
+       tt_data->flags = BATADV_TT_OGM_DIFF;
+-      if (tt_diff_len == 0)
++      if (!drop_changes && tt_diff_len == 0)
+               goto container_register;
+       spin_lock_bh(&bat_priv->tt.changes_list_lock);